Skip to content

Conversation

Fznamznon
Copy link
Contributor

Before using a constexpr variable that is not properly initialized check that it is valid.

Fixes #109095
Fixes #112516

@Fznamznon Fznamznon added this to the LLVM 19.X Release milestone Oct 18, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

Before using a constexpr variable that is not properly initialized check that it is valid.

Fixes #109095
Fixes #112516


Full diff: https://github.com/llvm/llvm-project/pull/112855.diff

2 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+7-3)
  • (modified) clang/test/Sema/constexpr.c (+14)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 490c4a2fc525cd..bc7cce0bcd7fc2 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2503,7 +2503,8 @@ bool VarDecl::isUsableInConstantExpressions(const ASTContext &Context) const {
   if (!DefVD->mightBeUsableInConstantExpressions(Context))
     return false;
   //   ... and its initializer is a constant initializer.
-  if (Context.getLangOpts().CPlusPlus && !DefVD->hasConstantInitialization())
+  if ((Context.getLangOpts().CPlusPlus || getLangOpts().C23) &&
+      !DefVD->hasConstantInitialization())
     return false;
   // C++98 [expr.const]p1:
   //   An integral constant-expression can involve only [...] const variables
@@ -2610,8 +2611,11 @@ bool VarDecl::hasICEInitializer(const ASTContext &Context) const {
 }
 
 bool VarDecl::hasConstantInitialization() const {
-  // In C, all globals (and only globals) have constant initialization.
-  if (hasGlobalStorage() && !getASTContext().getLangOpts().CPlusPlus)
+  // In C, all globals and constexpr variables should have constant
+  // initialization. For constexpr variables in C check that initializer is a
+  // constant initializer because they can be used in constant expressions.
+  if (hasGlobalStorage() && !getASTContext().getLangOpts().CPlusPlus &&
+      !isConstexpr())
     return true;
 
   // In C++, it depends on whether the evaluation at the point of definition
diff --git a/clang/test/Sema/constexpr.c b/clang/test/Sema/constexpr.c
index 8286cd2107d2f2..0bf3a41c18d76d 100644
--- a/clang/test/Sema/constexpr.c
+++ b/clang/test/Sema/constexpr.c
@@ -357,3 +357,17 @@ void infsNaNs() {
   constexpr double db5 = LD_SNAN; // expected-error {{constexpr initializer evaluates to nan which is not exactly representable in type 'const double'}}
   constexpr double db6 = INF;
 }
+
+void ghissue112516() {
+  struct S11 *s11 = 0;
+  constexpr int num = s11->len; // expected-error {{constexpr variable 'num' must be initialized by a constant expression}}
+  void *Arr[num];
+}
+
+void ghissue109095() {
+  constexpr char c[] = { 'a' };
+  constexpr int i = c[1]; // expected-error {{constexpr variable 'i' must be initialized by a constant expression}}\
+                          // expected-note {{declared here}}
+  _Static_assert(i == c[0]); // expected-error {{static assertion expression is not an integral constant expression}}\
+                             // expected-note {{initializer of 'i' is not a constant expression}}
+}

@tru
Copy link
Collaborator

tru commented Oct 28, 2024

@AaronBallman @shafik ?

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tru
Copy link
Collaborator

tru commented Oct 29, 2024

This has conflicts now. Can someone fix it and I can merge it for 19.1.4

@AaronBallman
Copy link
Collaborator

This has conflicts now. Can someone fix it and I can merge it for 19.1.4

I took a shot at resolving the conflicts (I think Mariya is out on vacation currently).

@tru
Copy link
Collaborator

tru commented Nov 12, 2024

I had to resolve some conflicts again - can you have a look @AaronBallman ?

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM!

@tru tru merged commit c9e8540 into llvm:release/19.x Nov 15, 2024
Before using a constexpr variable that is not properly initialized check
that it is valid.

Fixes llvm#109095
Fixes llvm#112516
Copy link

@Fznamznon (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category release:backport
Projects
Development

Successfully merging this pull request may close these issues.

5 participants